Schema Coordinates #794

magicmark · 2020-11-18T07:11:07Z

👋

I've pulled out the proposed spec edits for Schema Coordinates (issue: #735) into this PR.

(The RFC now lives in the original PR: https://github.com/graphql/graphql-spec/pull/746/files)

@leebyron you mentioned it might be possible to simplify the grammar - I played around a bit since the original PR, but I think I need some help here. What did you have in mind?

As suggested, tagging @eapache @mjmahone as additional reviewers as we had some good conversation in last WG meeting about this.

Thanks!

magicmark · 2020-11-18T07:15:50Z

Oh also, here's another option I had for the examples table, was thinking it might be good to explicitly show all the different kinds of schema you can select:

felt slightly too verbose to me, but curious to see if anyone prefers this

benjie

Thanks for your hard work on this!

spec/Appendix C -- Schema Coordinates.md

spec/Section 3 -- Type System.md

benjie

Looks good to me; a few optional tweaks suggested.

spec/Section 3 -- Type System.md

magicmark · 2021-01-04T08:14:35Z

spec/Section 3 -- Type System.md

+Schema Coordinates are always unique. Each type, field, argument, enum value, or
+directive may be referenced by exactly one possible Schema Coordinate.


feels like something worth highlighting and being clear about as a guarantee in the spec

magicmark · 2021-01-04T08:17:05Z

spec/Section 3 -- Type System.md

+For example, the following is *not* a valid Schema Coordinate:
+
+```graphql counter-example
+Entity.Business
+```
+
+In this counter example, both `Entity.Business` and `Business` would refer to
+the `Business` type. Since it would be ambiguous what the "primary key" should
+be in an application that uses Schema Coordinates to reference types, this is
+not allowed.


is this waffle? thought it might be helpful to have a little explanation/demonstration of this property

Could just be

- Since it would be ambiguous what the "primary key" should be in an application that uses Schema Coordinates to reference types, this is not allowed. + Such ambiguity is disallowed (and hence why this is is invalid syntax).

(More terse, but doesn't walk the reader through why this is bad)

Voting lines are now open!

In this counter example, Entity.Business is redundant since Business already uniquely identifies the Business type. Such redundancy is disallowed by this spec - every type, field, field argument, enum value, directive, and directive argument has exactly one canonical Schema Coordinate.

👍 thanks benjie, stealing this wording

Happy new year! 🥳 It's possible we still want more time for the RFC to marinate, but I'll attend so I can be available to folks to discuss the schema coordinates rfc/spec edit in person. graphql/graphql-spec#794 Thanks!

mcohen75 · 2021-02-04T18:10:35Z

At Indeed we log usage of schema elements to 1) support deprecation and 2) understand usage from a product management perspective (i.e. is this API we built adding value?).

We defined our own coordinate system that is not quite as complete as this spec. A nice benefit of formalizing a coordinate system is that shared tools can be built to solve these and other use cases.

👍 👍 👍

leebyron · 2021-04-14T01:01:00Z

I made some fairly significant edits to the prose and examples section in an attempt to simplify. @magicmark please take a look and let me know what you think.

leebyron · 2021-04-14T21:30:27Z

IIRC the only thing holding this from Approval stage is a landed GraphQL.js PR?

Implements graphql/graphql-spec#794 Adds: * DOT punctuator in lexer * Improvements to lexer errors around misuse of `.` * Minor improvement to parser core which simplified this addition * `SchemaCoordinate` node and `isSchemaCoodinate()` predicate * Support in `print()` and `visit()` * Added function `parseSchemaCoordinate()` since it is a parser entry point. * Added function `resolveSchemaCoordinate()` and `resolveASTSchemaCoordinate()` which implement the semantics (name mirrored from `buildASTSchema`) as well as the return type `ResolvedSchemaElement`

PascalSenn · 2021-12-21T21:43:16Z

spec/Section 3 -- Type System.md

+  3. If {type} is an Enum type:
+    1. Let {enumValueName} be the value of the second {Name}.
+    2. Return the enum value of {type} named {enumValueName}.
+  4. Otherwise if {type} is an Input Object type:
+    1. Let {inputFieldName} be the value of the second {Name}.
+    2. Return the input field of {type} named {inputFieldName}.
+  5. Otherwise:
+    1. Assert {type} must be an Object or Interface type.
+    2. Let {fieldName} be the value of the second {Name}.
+    3. Return the field of {type} named {fieldName}.


I think this section should also specify the behaviour for Union types.
At the moment the spec does not define the behaviour for Union.UnionTypeMember

Suggested change

3. If {type} is an Enum type:

1. Let {enumValueName} be the value of the second {Name}.

2. Return the enum value of {type} named {enumValueName}.

4. Otherwise if {type} is an Input Object type:

1. Let {inputFieldName} be the value of the second {Name}.

2. Return the input field of {type} named {inputFieldName}.

5. Otherwise:

1. Assert {type} must be an Object or Interface type.

2. Let {fieldName} be the value of the second {Name}.

3. Return the field of {type} named {fieldName}.

3. If {type} is an Union type:

1. Assert {type} must be an Enum, Input Object, Object or Interface type.

4. If {type} is an Enum type:

1. Let {enumValueName} be the value of the second {Name}.

2. Return the enum value of {type} named {enumValueName}.

5. Otherwise if {type} is an Input Object type:

1. Let {inputFieldName} be the value of the second {Name}.

2. Return the input field of {type} named {inputFieldName}.

6. Otherwise:

1. Assert {type} must be an Object or Interface type.

2. Let {fieldName} be the value of the second {Name}.

3. Return the field of {type} named {fieldName}.

Perhaps I'm misunderstanding your question, but to clarify:

UnionTypeMember

The definition for this would live somewhere else in the schema, outside of the context of the union. You would write the schema coordinate to that instead.

i.e. Union.UnionTypeMember is not a valid schema coordinate.

The rule of thumb is: there should only be one way to reference a thing with Schema Coordinates

Is there still some ambiguity in this spec?

@magicmark Makes sense to me

@leebyron is it worth keeping in the clarification (or something similar) from a previous commit?

https://github.com/magicmark/graphql-spec/blob/ac5318f9a9f04d0f8dd03d3758d5ca2654f7f7d7/spec/Section%203%20--%20Type%20System.md?plain=1#L391-L399

My gut says this may crop up again and cause some confusion - worth the counter example?

@leebyron is it worth keeping in the clarification (or something similar) from a previous commit?

magicmark/graphql-spec@ac5318f/spec/Section%203%20--%20Type%20System.md?plain=1#L391-L399

My gut says this may crop up again and cause some confusion - worth the counter example?

Just chiming in. From my perspective the counter example would be ideal (maybe even clarify that directives on types are also invalid?).

Maybe add a line on why union members are not valid schema coordinates, instead of solely stating that they are not?

- Note: You may not select members inside a union definition. + Note: Union members do not uniquely identify elements in the schema. Therefore the following counter example is *not* considered a valid Schema Coordinate:

...or something along those lines

fwiw this is all sort of encapsulated by the following line the spec:

A schema coordinate is always unique. Each schema element may be referenced
by exactly one possible schema coordinate.

++ for the explainer, I would maybe try and reuse this language (e.g. "as stated above, each schema element...."), or hyperlink back to this source of truth somehow

schenkman · 2022-10-12T17:16:04Z

Regarding input arguments, the example is shown as:

Query.searchBusiness(name:)

What would it look like for more interesting sub-input-types on an argument? Example:

input BazInput {
  count: Int
  color: String
}
input BarInput {
  baz_settings: BazInput
}
input FooInput {
  bar_params: BarInput
}

type Query {
  foos(foo_params: FooInput) 
}

Would the above translate to the following coordinates?

Query.foos(foo_params:)
Query.foos(foo_params:bar_params)
Query.foos(foo_params:bar_params:baz_settings:)
Query.foos(foo_params:bar_params:baz_settings:count:)
Query.foos(foo_params:bar_params:baz_settings:color:)

PascalSenn · 2022-10-13T05:37:14Z

@schenkman

This would be:

Query.foos(foo_params:) => Query.foos(foo_params:)
Query.foos(foo_params:bar_params) => FooInput.bar_params
Query.foos(foo_params:bar_params:baz_settings:) => BarInput.baz_settings
Query.foos(foo_params:bar_params:baz_settings:count:) => BazInput.count

benjie · 2022-10-13T06:19:53Z

Indeed, each schema entity should have exactly one schema coordinate. If you want to identify those kinds of paths in an operation (rather than the schema), then operation expressions might be what you’re after: https://github.com/graphql/graphql-wg/blob/main/rfcs/OperationExpressions.md#arguments

Implements graphql/graphql-spec#794 Adds: * DOT punctuator in lexer * Improvements to lexer errors around misuse of `.` * Minor improvement to parser core which simplified this addition * `SchemaCoordinate` node and `isSchemaCoodinate()` predicate * Support in `print()` and `visit()` * Added function `parseSchemaCoordinate()` since it is a parser entry point. * Added function `resolveSchemaCoordinate()` and `resolveASTSchemaCoordinate()` which implement the semantics (name mirrored from `buildASTSchema`) as well as the return type `ResolvedSchemaElement`

ddebrunner · 2023-12-04T17:36:02Z

@PascalSenn @benjie Trying to understand the last two comments:

This would be:
Query.foos(foo_params:bar_params) => FooInput.bar_params

Q1 - "This would be" - intended to mean this is direction the spec will go as it's not covered in this PR?

I assume it would also apply to directives, so with @A(foo_params: FooInput) then the coordinate for bar_params would be:

@A(foo_params:bar_params)

I saw the comment here: https://github.com/graphql/graphql-wg/blob/main/rfcs/SchemaCoordinates.md?plain=1#L370-L375 but it throws me a little as it talks about supporting more than "schema coordinates" but then the first example seems to be a schema coordinate Foo.bar(baz.qux:)

Also note the example Foo.bar(baz.qux:) is different to the comment above, which would have it as Foo.bar(baz:qux) but the latter seems to make more sense.

benjie · 2023-12-04T23:00:30Z

@ddebrunner Schema Coordinates and Operation Expressions are two separate proposals with related syntax.

Every entity in a schema has exactly one schema coordinate, there is no schema coordinate @A(foo_params:bar_params:), at least not as currently proposed.

The property bar_params on the type FooInput has the schema coordinate FooInput.bar_params, independent of whether it's accessed through a directive (@A(foo_params:{bar_params:...})) or a field (myField(foo_params:{bar_params:...})).

You can, however, use Operation Expressions to indicate the accessing of something in this manner, since Operation Expressions are not unique.

The comment you quote relates to (what became) Operation Expressions, which is a system that builds upon the Schema Coordinates syntax to talk about paths within operations (rather than the schema).

ddebrunner · 2023-12-05T13:33:09Z

@benjie I'm not talking about operations, but schema elements. A type system directive is applied to a schema element and thus is also a schema element(?), so what are the coordinates to such arguments?

E.g.

input IO {
  a:IO2
}
input IO2 {
  b:Int
}
directive @x(v:IO)

type T {
   f:Int @x(v:{a:{b:3}})
}

Now if the value of b is invalid how would I report the coordinate of b in an error message.
The comments related to this RFC seem to imply something like:

@x(v:a.b) on T.f

But I'm assuming that the use of type system directive@x is a schema element, and therefore so are its arguments.
Maybe I'm missing something?

benjie · 2023-12-05T13:45:44Z

I don't think the application of a directive is covered by this proposal, only the directive itself. At runtime your @x application on T.f no longer exists - e.g. if you introspect that schema or use graphql.printSchema(schema) you'll not see it - it's a build-time-only directive currently. (See #300 for more details.)

You could use operation expressions to indicate the path of this directive application, I guess it's more "document expressions" than "operation expressions" thinking about it.

ddebrunner · 2023-12-05T17:25:56Z

Fair enough, but I hadn't assumed that the RFC was related to runtime, thus the fact that introspection doesn't return type system directives would seem to be irrelevant to the RFC.

Implements graphql/graphql-spec#794 Adds: * DOT punctuator in lexer * Improvements to lexer errors around misuse of `.` * Minor improvement to parser core which simplified this addition * `SchemaCoordinate` node and `isSchemaCoodinate()` predicate * Support in `print()` and `visit()` * Added function `parseSchemaCoordinate()` since it is a parser entry point. * Added function `resolveSchemaCoordinate()` and `resolveASTSchemaCoordinate()` which implement the semantics (name mirrored from `buildASTSchema`) as well as the return type `ResolvedSchemaElement`

magicmark mentioned this pull request Nov 18, 2020

RFC: Schema Coordinates #746

Merged

benjie requested changes Nov 18, 2020

View reviewed changes

spec/Appendix C -- Schema Coordinates.md Outdated Show resolved Hide resolved

spec/Appendix C -- Schema Coordinates.md Outdated Show resolved Hide resolved

leebyron reviewed Nov 19, 2020

View reviewed changes

spec/Appendix C -- Schema Coordinates.md Outdated Show resolved Hide resolved

spec/Appendix C -- Schema Coordinates.md Outdated Show resolved Hide resolved

spec/Appendix C -- Schema Coordinates.md Outdated Show resolved Hide resolved

magicmark commented Nov 21, 2020

View reviewed changes

spec/Section 3 -- Type System.md Outdated Show resolved Hide resolved

benjie approved these changes Nov 23, 2020

View reviewed changes

spec/Section 3 -- Type System.md Outdated Show resolved Hide resolved

spec/Section 3 -- Type System.md Outdated Show resolved Hide resolved

spec/Section 3 -- Type System.md Outdated Show resolved Hide resolved

spec/Section 3 -- Type System.md Outdated Show resolved Hide resolved

leebyron reviewed Dec 3, 2020

View reviewed changes

spec/Section 3 -- Type System.md Outdated Show resolved Hide resolved

magicmark commented Jan 4, 2021

View reviewed changes

magicmark mentioned this pull request Jan 4, 2021

Add Mark to January WG meeting graphql/graphql-wg#568

Merged

leebyron added the 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) label Jan 7, 2021

Base automatically changed from master to main February 3, 2021 04:50

leebyron added 📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md) and removed 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) labels Mar 4, 2021

michaelstaib mentioned this pull request Mar 4, 2021

Track: Schema Coordinates: add spec edit ChilliCream/graphql-platform#3193

Closed

leebyron added this to the May2021 milestone Apr 13, 2021

leebyron force-pushed the schema_coordinates_spec_edit branch 3 times, most recently from 4f854af to c0b82d5 Compare April 13, 2021 23:57

leebyron force-pushed the schema_coordinates_spec_edit branch 3 times, most recently from 81ef020 to 4299d8c Compare April 14, 2021 01:18

leebyron requested review from benjie and a team April 14, 2021 01:20

leebyron changed the title ~~Schema Coordinates: add spec edit~~ Schema Coordinates Apr 14, 2021

leebyron approved these changes Apr 14, 2021

View reviewed changes

leebyron force-pushed the main branch from de2a1ff to 51337a9 Compare October 26, 2021 23:50

PascalSenn reviewed Dec 21, 2021

View reviewed changes

pcmanus mentioned this pull request Mar 25, 2022

Allows @inacessible in subgraphs apollographql/federation#1638

Merged

sungam3r mentioned this pull request Jun 22, 2022

Replace authorization project with new rule graphql-dotnet/server#803

Merged

Shane32 mentioned this pull request Jun 22, 2022

Consider using schema coordinates syntax for access denied errors graphql-dotnet/server#835

Open

yaacovCR mentioned this pull request Jan 2, 2023

RFC: Schema Coordinates graphql/graphql-js#3807

Open

benjie mentioned this pull request Jan 20, 2023

Standardize naming for "Schema Coordinates" (e.g. Foo.bar) #735

Open

benjie mentioned this pull request Feb 8, 2024

More GraphQL native expression for FieldSelection? graphql/composite-schemas-spec#21

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Schema Coordinates #794

Schema Coordinates #794

magicmark commented Nov 18, 2020

magicmark commented Nov 18, 2020 •

edited

Loading

benjie left a comment

benjie left a comment

magicmark Jan 4, 2021

magicmark Jan 4, 2021

magicmark Jan 4, 2021 •

edited

Loading

benjie Jan 5, 2021

magicmark Jan 7, 2021

mcohen75 commented Feb 4, 2021

leebyron commented Apr 14, 2021

leebyron commented Apr 14, 2021

PascalSenn Dec 21, 2021

magicmark Dec 30, 2021 •

edited

Loading

PascalSenn Dec 31, 2021

magicmark Dec 31, 2021 •

edited

Loading

jord1e Dec 31, 2021

magicmark Dec 31, 2021 •

edited

Loading

schenkman commented Oct 12, 2022 •

edited

Loading

PascalSenn commented Oct 13, 2022

benjie commented Oct 13, 2022

ddebrunner commented Dec 4, 2023 •

edited

Loading

benjie commented Dec 4, 2023 •

edited

Loading

ddebrunner commented Dec 5, 2023 •

edited

Loading

benjie commented Dec 5, 2023

ddebrunner commented Dec 5, 2023

		Schema Coordinates are always unique. Each type, field, argument, enum value, or
		directive may be referenced by exactly one possible Schema Coordinate.

Schema Coordinates #794

Are you sure you want to change the base?

Schema Coordinates #794

Conversation

magicmark commented Nov 18, 2020

magicmark commented Nov 18, 2020 • edited Loading

benjie left a comment

Choose a reason for hiding this comment

benjie left a comment

Choose a reason for hiding this comment

magicmark Jan 4, 2021

Choose a reason for hiding this comment

magicmark Jan 4, 2021

Choose a reason for hiding this comment

magicmark Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

benjie Jan 5, 2021

Choose a reason for hiding this comment

magicmark Jan 7, 2021

Choose a reason for hiding this comment

mcohen75 commented Feb 4, 2021

leebyron commented Apr 14, 2021

leebyron commented Apr 14, 2021

PascalSenn Dec 21, 2021

Choose a reason for hiding this comment

magicmark Dec 30, 2021 • edited Loading

Choose a reason for hiding this comment

PascalSenn Dec 31, 2021

Choose a reason for hiding this comment

magicmark Dec 31, 2021 • edited Loading

Choose a reason for hiding this comment

jord1e Dec 31, 2021

Choose a reason for hiding this comment

magicmark Dec 31, 2021 • edited Loading

Choose a reason for hiding this comment

schenkman commented Oct 12, 2022 • edited Loading

PascalSenn commented Oct 13, 2022

benjie commented Oct 13, 2022

ddebrunner commented Dec 4, 2023 • edited Loading

benjie commented Dec 4, 2023 • edited Loading

ddebrunner commented Dec 5, 2023 • edited Loading

benjie commented Dec 5, 2023

ddebrunner commented Dec 5, 2023

magicmark commented Nov 18, 2020 •

edited

Loading

magicmark Jan 4, 2021 •

edited

Loading

magicmark Dec 30, 2021 •

edited

Loading

magicmark Dec 31, 2021 •

edited

Loading

magicmark Dec 31, 2021 •

edited

Loading

schenkman commented Oct 12, 2022 •

edited

Loading

ddebrunner commented Dec 4, 2023 •

edited

Loading

benjie commented Dec 4, 2023 •

edited

Loading

ddebrunner commented Dec 5, 2023 •

edited

Loading